CHANGE: Split Runtime and Editor code into separate folders to prepare for Module migration#2321
Open
jfreire-unity wants to merge 104 commits intodevelopfrom
Open
CHANGE: Split Runtime and Editor code into separate folders to prepare for Module migration#2321jfreire-unity wants to merge 104 commits intodevelopfrom
jfreire-unity wants to merge 104 commits intodevelopfrom
Conversation
- Move InputSystem/* to Runtime/ (preserving history) - Move InputSystem/Editor to Editor/ (preserving history) - Add meta files for new folder structure - Follows Unity package layout conventions - All file history preserved via git mv
Also exposes the required internals between assemblies.
This is done to avoid calling into Edito specific code. Instead, it will be called if Editor code has registered a callback.
Due to refactoring, a lot of paths still contained the InputSystem folder path that no longer exists. We only have Editor and Runtime folders.
jfreire-unity
commented
Jan 27, 2026
Packages/com.unity.inputsystem/Editor/Plugins/HID/HIDSupportEditorInitializer.cs
Outdated
Show resolved
Hide resolved
Contributor
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Contributor
PR Code Suggestions ✨🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr |
Some files were moved to other folders, after merging conflicts arise but since they showed as removed changes didn't land on the moved files. This commit is fixing this
Collaborator
Author
|
/crc |
Contributor
|
This PR introduces several critical issues that need to be addressed, including a potential compilation error in 🤖 Helpful? 👍/👎 |
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
| @@ -216,13 +216,13 @@ public static Command DeleteSelectedControlScheme() | |||
| if (indexOfArrayElement > serializedArray.arraySize - 1) | |||
| return state.With( | |||
| selectedControlSchemeIndex: serializedArray.arraySize - 1, | |||
| selectedControlScheme: new InputControlScheme(serializedArray.GetArrayElementAtIndex(serializedArray.arraySize - 1)), selectedDeviceRequirementIndex: -1); | |||
| selectedControlScheme: InputControlSchemeEditorExtensions.FromSerializedProperty(serializedArray.GetArrayElementAtIndex(serializedArray.arraySize - 1)), selectedDeviceRequirementIndex: -1); | |||
Contributor
There was a problem hiding this comment.
This logic will cause a crash when the last remaining control scheme is deleted. If serializedArray.arraySize becomes 0 after the deletion, the condition indexOfArrayElement > serializedArray.arraySize - 1 (which resolves to 0 > -1) will be true, leading to a call to serializedArray.GetArrayElementAtIndex(-1). This will throw an IndexOutOfRangeException.
🤖 Helpful? 👍/👎
Contributor
There was a problem hiding this comment.
To prevent the IndexOutOfRangeException when the last control scheme is deleted (resulting in an array size of 0), you should explicitly handle the empty case.
I suggest checking the array size before attempting to access an index. If the array is empty, set the selection to -1 and the scheme to default:
if (serializedArray.arraySize == 0)
{
return state.With(
selectedControlSchemeIndex: -1,
selectedControlScheme: default,
selectedDeviceRequirementIndex: -1);
}
if (indexOfArrayElement >= serializedArray.arraySize)
{
var lastIndex = serializedArray.arraySize - 1;
return state.With(
selectedControlSchemeIndex: lastIndex,
selectedControlScheme: InputControlSchemeEditorExtensions.FromSerializedProperty(serializedArray.GetArrayElementAtIndex(lastIndex)),
selectedDeviceRequirementIndex: -1);
}This ensures that if serializedArray.arraySize is 0, the first block returns early and we never call GetArrayElementAtIndex(-1).
🤖 Helpful? 👍/👎
Collaborator
Author
There was a problem hiding this comment.
@u-pr was this problem not there before the change this PR introduces?
Contributor
There was a problem hiding this comment.
Yes, the problem existed before this PR. Looking at the code before your changes:
// Original code
if (indexOfArrayElement > serializedArray.arraySize - 1)
return state.With(
selectedControlSchemeIndex: serializedArray.arraySize - 1,
selectedControlScheme: new InputControlScheme(serializedArray.GetArrayElementAtIndex(serializedArray.arraySize - 1)),
selectedDeviceRequirementIndex: -1);If the last control scheme was deleted and serializedArray.arraySize became 0, the condition indexOfArrayElement > -1 would still evaluate to true (since the previous index was 0). This would lead to GetArrayElementAtIndex(-1), causing the same exception.
Your PR swapped new InputControlScheme(...) for InputControlSchemeEditorExtensions.FromSerializedProperty(...), but the underlying indexing logic remained the same. This is a pre-existing bug that was likely inherited or overlooked in the original implementation.
🤖 Helpful? 👍/👎
Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorState.cs
Show resolved
Hide resolved
| #endif | ||
| } | ||
|
|
||
| var existingStateManagers = Resources.FindObjectsOfTypeAll<InputSystemStateManager>(); |
Contributor
There was a problem hiding this comment.
Using Resources.FindObjectsOfTypeAll<InputSystemStateManager>() to retrieve the state manager after a domain reload is functional, but have you considered using ScriptableSingleton<InputSystemStateManager> instead? This would align with the pattern used for RemoteInputPlayerConnectionEditor in this PR and would provide a more robust and idiomatic way to handle persistent editor-only state without the overhead of searching all objects.
🤖 Helpful? 👍/👎
jfreire-unity
changed the title
Essentially, the monolitch assembly Unity.InputSystem was added back. The folder structure was kept so that the Runtime code is decoupled from Editor dependencies. This will help us move into a module in trunk and helps doing backports in the future, as well as current forward ports until the team is all onboarded into trunk.
There's no Editor assembly anymore so we need to make sure this code is not compiled when building players.
jfreire-unity
force-pushed
the
joao/editor-runtime-split-for-module
branch
from
c11a539 to
9c3e951
Compare
Update pvp-exemptions.json for pvp: 92-3
Removed NDA Keyword Validation entries from ValidationExceptions.json.
K-Tone
reviewed
Apr 8, 2026
| UnityEngine.TestTools.LogAssert.Expect(LogType.Error, "ScriptableSingleton already exists. Did you query the singleton in a constructor?"); | ||
| UnityEngine.TestTools.LogAssert.Expect(LogType.Error, "ScriptableSingleton already exists. Did you query the singleton in a constructor?"); | ||
| // Ensure the singleton is initialized deterministically in editor. | ||
| _ = RemoteInputPlayerConnection.instance; |
Collaborator
There was a problem hiding this comment.
what is the the underscore thing here in this expression?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Background & Goal
NOTE 🤯: This PR is huge and hard to review, but a lot of the changes are moving files and folders around. I know it's not easy to navigate these huge changes so I tried my best to do a good PR description.
The Unity Input System package historically had
UnityEditor.*references scattered throughout its runtime code, guarded by#if UNITY_EDITORpreprocessor directives, but still in the same files as runtime logic.This was making the codebase hard to port into a Unity module in trunk (
unity/unity), and it was also harder to reason about the Editor vs Standalone player boundary.This PR tries to break the dependency of Editor references in Runtime code to help us migrate to a Unity module and does the following changes:
UnityEditor.*references remain in the runtime source files.Unity.InputSystemassembly is kept . The folder reorganisation (Runtime/andEditor/subfolders) is the externally visible result, making forward-ports and movinggithistory to trunk and eventual module migration easier. We decided not to introduce 2 assemblies as this would duplicate our upgrade path when projects upgrade to a Unity version using the new Input System module.Ideally, we would have liked to also have a Runtime and Editor assembly.
That work was initially done but then we reverted it due to the last point above.
This PR was done with the help of agentic coding as well.
High-Level Changes
InputSystem/Runtime/, all editor source toInputSystem/Editor/. 1,201 files renamed (history preserved viagit mv).Unity.InputSystem.asmdefkept as single monolith. AUnity.InputSystem.Editorasmdef was prototyped and then rolled back — the folder split remains as foundation for a future module migration.UnityEditor.*call in runtime files with internalstaticcallback fields (Action,Func). The Editor registers these at startup via[InitializeOnLoad]. In player builds these fields arenulland all call sites are null-safe (?.Invoke()).InputSystemEditorInitializer.cs[InitializeOnLoad]class inEditor/that owns all editor lifecycle concerns previously scattered across the runtime (play-mode changes, domain reload, asset tracking, analytics, Unity Remote, remoting, etc.).InputSystemState.csInputSystemObjectand editor-specific paths) into its own runtime class to help with the dependency boundary.*Editorclasses (Interaction editors, Composite editors, Processor editors, OnScreen editors) removed from runtime files and placed in dedicated files underEditor/.#if UNITY_EDITORmoved toEditor/(e.g.PlayerInputEditor,UnityRemoteSupport,iOSPostProcessBuild).InputTestRuntime,InputTestStateManagerupdated to reflect the new interface contracts (e.g.onPlayModeChanged: Action<int>instead ofAction<PlayModeStateChange>).Pattern used for these changes
The approach used throughout this PR is maybe worth understanding before reviewing individual files:
This pattern appears in:
InputSystem.cs,InputManager.cs,NativeInputRuntime.cs,InputActionSetupExtensions.cs,InputActionAsset.cs,InputActionReference.cs,EnhancedTouchSupport.cs,RemoteInputPlayerConnection.cs,OnScreenStick.cs,TrackedPoseDriver.cs,InputSystemUIInputModule.cs.Review Guide Suggestions
1.
InputSystem/Runtime/InputSystem.csusing UnityEditor;is gone.s_OnSettingsChanged,s_OnActionsChanging,s_ShouldEnableActions,s_OnPlayModeChangeCallback) are used consistently and null-safely.#if UNITY_EDITORblock forOnPlayModeChangeis intentional — it is a forwarding shim for tests that call it directly.2.
InputSystem/Runtime/NativeInputRuntime.csEditorApplication.isPlaying,EditorApplication.isPaused,InternalEditorUtility.isApplicationActive, andEditorAnalyticscalls.internal boolfields (m_IsInPlayMode,m_IsEditorPaused,m_IsEditorActive) and callbacks (m_SendEditorAnalytic,m_SetUnityRemoteMessageHandler, etc.).UpdateEditorState()method inInputSystemEditorInitializeris pumping those fields onEditorApplication.update.3.
InputSystem/Runtime/InputManager.csEditorApplication.isPaused→m_Runtime.isEditorPaused(fromIInputRuntime).InputSystem.s_SystemObjectreferences replaced with localm_ExitEditModeTime/m_EnterPlayModeTimefields.ProjectWideActionsBuildProvider.actionsToIncludeInPlayerBuildreplaced withs_GetProjectWideActionscallback.4.
InputSystem/Runtime/IInputRuntime.csonPlayModeChangedchanged fromAction<PlayModeStateChange>toAction<int>— removes theUnityEditorenum from the interface.bool isEditorPaused { get; }.5.
InputSystem/Editor/InputSystemEditorInitializer.cs(new file)OnEditorPlayModeStateChanged→ setsm_IsInPlayMode,m_IsEditorPaused, dispatchesDispatchPlayModeChange, etc.InputSystemStateis loaded/restored here.6. Extracted
*Editorclasses (Editor/Actions/Interactions/,Editor/Actions/Composites/,Editor/Controls/Processors/,Editor/Plugins/OnScreen/)7.
InputSystem/Runtime/Actions/InputActionReference.cs,InputActionAsset.cs,EnhancedTouchSupport.csAssetDatabase/AssemblyReloadEventscalls replaced with callback fields.#if UNITY_EDITOR-guarded.8. Test fixtures —
Tests/TestFixture/InputTestRuntime.cs:onPlayModeChangedis nowAction<int>— make sure test call sites cast correctly.InputTestStateManager.cs: there were some naming changes, so something to be aware.9. Assembly /
.asmdeffilesUnity.InputSystem.asmdefshould still reference a single assembly (no Editor split).Unity.InputSystemonly.AssemblyInfo.cs:InternalsVisibleToentries cover test assemblies — verify no entry was accidentally dropped.InputSystemForUI.Editor.asmdef, since the previous folder containingInputSystemForUI.asmdefwas split into Editor and Runtime. This is part of internal API so I don't see any breakages in this area when upgrading.Testing status & QA
Built and run Samples on both Windows and macOS Play-mode and Standalone builds.
I recommend validating all the Samples work and do kind of the sames tests as we would do for a Release since this PR touches a lot of things.
Overall Product Risks
Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.
Comments to reviewers
Some sanity check guides:
using UnityEditor;in any file underInputSystem/Runtime/InputSystemEditorInitializerregisters every callback that has a field on the runtime sideInputTestRuntimecompiles and tests pass with the updatedAction<int>signatureUnity.InputSystemwas accidentally wired to a now-deletedUnity.InputSystem.Editorassembly. Hopefully there are not left overs of this initial work.One thing is not in place is a mechanism to guarantee we don't leak UnityEditor references in the Runtime folder. I'll follow up on that in another PR.
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.